Skip to content

Remove DacDbi GetAppDomainObject API from native and cDAC#127673

Merged
barosiak merged 3 commits intodotnet:mainfrom
barosiak:barosiak/GetAppDomainObject
May 2, 2026
Merged

Remove DacDbi GetAppDomainObject API from native and cDAC#127673
barosiak merged 3 commits intodotnet:mainfrom
barosiak:barosiak/GetAppDomainObject

Conversation

@barosiak
Copy link
Copy Markdown
Member

@barosiak barosiak commented May 1, 2026

GetRawExposedObjectHandleForDebugger() always returned NULL, making GetAppDomainObject a no-op.
This pr removes it from all layers:

  • IDacDbiInterface (abstract interface + IDL)
  • DacDbiInterfaceImpl (native implementation)
  • cDAC managed interface and implementation
  • AppDomain::GetRawExposedObjectHandleForDebugger (sole consumer)
  • CordbAppDomain::GetObject call site (simplified, since deletion created new dead code)

@barosiak barosiak requested review from max-charlamb, noahfalk and rcj1 May 1, 2026 22:28
@barosiak barosiak self-assigned this May 1, 2026
Copilot AI review requested due to automatic review settings May 1, 2026 22:28
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the unused GetAppDomainObject API across the DAC/DBI native COM contract and the legacy managed cDAC projection, since the underlying GetRawExposedObjectHandleForDebugger() always returned NULL and the right-side API effectively always returned S_FALSE.

Changes:

  • Deleted GetAppDomainObject from the IDacDbiInterface contract (IDL + native abstract interface + native implementation).
  • Removed the legacy managed cDAC projection/forwarder for GetAppDomainObject.
  • Simplified CordbAppDomain::GetObject now that the underlying DAC query path is gone.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/IDacDbiInterface.cs Removes the managed COM projection method for GetAppDomainObject.
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs Removes legacy-fallback forwarding implementation for GetAppDomainObject.
src/coreclr/vm/appdomain.hpp Removes AppDomain::GetRawExposedObjectHandleForDebugger() (sole consumer of the removed DAC API).
src/coreclr/inc/dacdbi.idl Removes GetAppDomainObject from the COM IDL contract.
src/coreclr/debug/inc/dacdbiinterface.h Removes GetAppDomainObject from the native abstract interface definition/docs.
src/coreclr/debug/di/rsappdomain.cpp Simplifies CordbAppDomain::GetObject to always return S_FALSE/null.
src/coreclr/debug/daccess/dacdbiimpl.h Removes GetAppDomainObject declaration from DAC implementation class.
src/coreclr/debug/daccess/dacdbiimpl.cpp Removes GetAppDomainObject implementation from the DAC.

Comment thread src/coreclr/inc/dacdbi.idl
Comment thread src/coreclr/debug/di/rsappdomain.cpp
Copilot AI review requested due to automatic review settings May 1, 2026 23:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/inc/dacdbi.idl
@barosiak barosiak enabled auto-merge (squash) May 1, 2026 23:29
@hoyosjs
Copy link
Copy Markdown
Member

hoyosjs commented May 2, 2026

/ba-g Scheduler issue for NAOT win-x64 debug. Leg isn't affected by this change.

@barosiak barosiak merged commit fcfb4ae into dotnet:main May 2, 2026
117 of 121 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants